Skip to content

Conversation

@si-zero
Copy link
Contributor

@si-zero si-zero commented Aug 21, 2025

Issue

변경 내용

  • 임원진 멤버 조회, 생성, 수정, 삭제 기능 추가

테스트

  • 로컬에서 테스트 완료
  • 기존 기능에 영향 없음 확인

리뷰 요구사항

  • 에러 코드를 enum 을 파서 만들고 싶은데, 어떤식으로 정의할까요?

@si-zero si-zero requested review from dohy-eon, hodoon and ysw789 August 21, 2025 06:37
@si-zero si-zero self-assigned this Aug 21, 2025
@ysw789
Copy link
Member

ysw789 commented Aug 21, 2025

작업하느라 고생 많으셨습니다! 부가적인 리뷰 남겨드리겠습니다.

  1. 현재 임원진 조회에 '전체 조회'는 없는 것 같은데 확인해주세요.
  2. 임원진 생성, 수정 API에 응답값이 필수가 아니라면 없어도 될 것 같습니다. 이 부분은 프론트엔드 파트와의 논의가 필요해보입니다.
  3. 임원진 조회 정렬 기준이 필요해보입니다. 임원진 목록 페이지에서는 보통 직책을 기준으로 그룹화 및 정렬이 이루어져야할텐데, ExecutiveEntity에 정렬 기준(sortOrder) 필드를 추가해 프론트엔드에서 해당 값을 기준으로 정렬하도록 기준을 제공해주면 좋을 것 같습니다.
  4. 조회 이외의 작업 (생성, 수정, 삭제)는 특정 권한을 가진 사용자(예를들어 ADMIN)만 접근 가능하도록 권한을 체크해야할 것 같습니다.

@ysw789
Copy link
Member

ysw789 commented Aug 21, 2025

에러 코드를 enum은 ErrorCode.java 에 정의되어있으므로 추가해서 사용하시면 됩니다!

Copy link
Member

@dohy-eon dohy-eon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고 많으셨습니다! 아래에 개선방안 몇가지만 작성해두겠습니다!

  1. 컨트롤러 스펠링 오탈자 수정
    ExecutiveContorller -> ExecutiveController

  2. 예외처리
    기존 IllegalArgumentException("Executive not found")

[ 권장 수정 방안 ]
.orElseThrow(() -> new CustomException(ErrorCode.EXECUTIVE_NOT_FOUND));

ErrorCode에 EXECUTIVE_NOT_FOUND(HttpStatus.NOT_FOUND, "임원진을 찾을 수 없습니다.");
추가 후 사용

추가로 테스트 코드에 성공 케이스만 존재하던데 실패 케이스도 추가하면 좋을 것 같습니다!

고생하셨습니다 ☺️

@jucheonsu jucheonsu linked an issue Aug 22, 2025 that may be closed by this pull request
@si-zero si-zero changed the base branch from main to dev August 23, 2025 05:09
@si-zero si-zero force-pushed the feat/DASOMBE-14-organization branch from 82d1f47 to 3375bcc Compare August 26, 2025 10:53
@si-zero
Copy link
Contributor Author

si-zero commented Aug 26, 2025

@ysw789 @dohy-eon @hodoon
코드 수정해서 올렸습니다!

Copy link
Member

@dohy-eon dohy-eon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니당

@si-zero si-zero merged commit d884a28 into dev Aug 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] 조직도 API 생성

5 participants